Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make it possible to disable ctrlf-mode locally in buffers #53

Merged
merged 2 commits into from
Jun 26, 2020
Merged

Make it possible to disable ctrlf-mode locally in buffers #53

merged 2 commits into from
Jun 26, 2020

Conversation

andersjohansson
Copy link
Contributor

Do this by defining ctrlf-global-mode and let ctrlf-mode be an
ordinary buffer-local minor-mode

See this more as a suggestion, one way of solving problems like #52

raxod502
raxod502 requested changes Jun 17, 2020
View reviewed changes
Copy link
Member

@raxod502 raxod502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is reasonable, but most people should be enabling ctrlf-mode globally. Can we make the global mode called ctrlf-mode and the local mode called ctrlf-local-mode?

ctrlf.el Outdated Show resolved Hide resolved
@andersjohansson andersjohansson force-pushed the make-disable-possible branch from 2fcb00b to 80ed834 Compare June 19, 2020 11:06
@andersjohansson
Copy link
Contributor Author

Yes, I also thought it would be more reasonable to keep ctrlf-modeas the global mode initially (this would also be backward-compatible for ctrlf). Then the most common convention in emacs is to call the give the global mode the global qualifier (very often as a prefix, breaking package namespacing) and not the other way around (though I did not find any specific recommendations for this in the documentation). But as the global mode is the primary expected way of using ctrlf, going with ctrlf-modeand ctrlf-local-modeseems fine to me.

@andersjohansson
Copy link
Contributor Author

I didn't update the readme now, since I was unsure if you wanted to do the blocklist thing as well. The only mode hook I know now where (ctrlf-local-mode -1) should be added is pdf-isearch-minor-mode-hook.

raxod502
raxod502 approved these changes Jun 25, 2020
View reviewed changes
Copy link
Member

@raxod502 raxod502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'd be happy to merge this with a small README update and CHANGELOG entry. You could also add the blacklist if you wanted, but it would probably make more sense to do that in a separate pull request.

@andersjohansson andersjohansson force-pushed the make-disable-possible branch from 80ed834 to 8159c39 Compare June 26, 2020 09:09
@andersjohansson andersjohansson changed the title WIP: Make it possible to disable ctrlf-mode locally in buffers Make it possible to disable ctrlf-mode locally in buffers Jun 26, 2020
@andersjohansson andersjohansson force-pushed the make-disable-possible branch from 8159c39 to e8b3266 Compare June 26, 2020 09:10
@andersjohansson
Copy link
Contributor Author

Hope this looks good!

@andersjohansson
Make it possible to disable ctrlf-mode locally in buffers
e49ad5e 
Do this by defining ctrlf-local-mode as a local minor mode and ctrlf-mode as
a global minor mode
@andersjohansson andersjohansson force-pushed the make-disable-possible branch from e8b3266 to e49ad5e Compare June 26, 2020 09:13
@raxod502
Copy link
Member

Commit 44155e3 improves the formatting. Otherwise, looks good. Thanks!

@raxod502 raxod502 merged commit 96ea55a into radian-software:master Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raxod502 raxod502 raxod502 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andersjohansson @raxod502